[WIP] Rename MaybeUninit::get_{ref,mut} to MaybeUninit::assume_init_{ref,mut} #66174

danielhenrymantilla · 2019-11-06T23:13:48Z

This is the last remaining concern blocking the stabilization of the by-ref accessors of MaybeUninit (#63568).
This change of name not only does align with .into_inner() being called .assume_init(),
it also conveys the dangerous semantics of the method in a much clearer and more direct way,
reducing the change of misuse and thus of unsoundness.

r? @RalfJung cc @SimonSapin @Centril

This is the last remaining concern blocking the stabilization of the by-ref accessors of `MaybeUninit` (rust-lang#63568). This change of name not only does align with `.into_inner()` being called `.assume_init()`, it also conveys the dangerous semantics of the method in a much clearer and more direct way, reducing the change of misuse and thus of unsoundness.

rust-highfive · 2019-11-06T23:21:24Z

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2019-11-06T23:14:26.9570064Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-11-06T23:14:26.9798492Z ##[command]git config gc.auto 0
2019-11-06T23:14:26.9869103Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-11-06T23:14:26.9916873Z ##[command]git config --get-all http.proxy
2019-11-06T23:14:27.0055080Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/66174/merge:refs/remotes/pull/66174/merge
---
2019-11-06T23:21:13.5923498Z    Compiling autocfg v0.1.6
2019-11-06T23:21:14.3262475Z error[E0599]: no method named `get_mut` found for type `mem::maybe_uninit::MaybeUninit<[u8; 1024]>` in the current scope
2019-11-06T23:21:14.3262949Z    --> src/libcore/fmt/float.rs:22:64
2019-11-06T23:21:14.3287725Z     |
2019-11-06T23:21:14.3288372Z 22  | ...                   false, buf.get_mut(), parts.get_mut());
2019-11-06T23:21:14.3288670Z     |                                  ^^^^^^^ method not found in `mem::maybe_uninit::MaybeUninit<[u8; 1024]>`
2019-11-06T23:21:14.3289279Z    ::: src/libcore/mem/maybe_uninit.rs:219:1
2019-11-06T23:21:14.3289464Z     |
2019-11-06T23:21:14.3289683Z 219 | pub union MaybeUninit<T> {
2019-11-06T23:21:14.3289985Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3289985Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3290172Z     |
2019-11-06T23:21:14.3290411Z     = help: items from traits can only be used if the trait is implemented and in scope
2019-11-06T23:21:14.3290653Z     = note: the following trait defines an item `get_mut`, perhaps you need to implement it:
2019-11-06T23:21:14.3291071Z             candidate #1: `slice::SliceIndex`
2019-11-06T23:21:14.3291243Z 
2019-11-06T23:21:14.3321578Z error[E0599]: no method named `get_mut` found for type `mem::maybe_uninit::MaybeUninit<[num::flt2dec::Part<'_>; 4]>` in the current scope
2019-11-06T23:21:14.3322762Z    --> src/libcore/fmt/float.rs:22:81
2019-11-06T23:21:14.3323875Z     |
2019-11-06T23:21:14.3325978Z 22  | ...                   false, buf.get_mut(), parts.get_mut());
2019-11-06T23:21:14.3327375Z     |                                                   ^^^^^^^ method not found in `mem::maybe_uninit::MaybeUninit<[num::flt2dec::Part<'_>; 4]>`
2019-11-06T23:21:14.3329780Z    ::: src/libcore/mem/maybe_uninit.rs:219:1
2019-11-06T23:21:14.3330698Z     |
2019-11-06T23:21:14.3331941Z 219 | pub union MaybeUninit<T> {
2019-11-06T23:21:14.3333563Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3333563Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3335443Z     |
2019-11-06T23:21:14.3337055Z     = help: items from traits can only be used if the trait is implemented and in scope
2019-11-06T23:21:14.3338679Z     = note: the following trait defines an item `get_mut`, perhaps you need to implement it:
2019-11-06T23:21:14.3339892Z             candidate #1: `slice::SliceIndex`
2019-11-06T23:21:14.3407008Z error[E0599]: no method named `get_mut` found for type `mem::maybe_uninit::MaybeUninit<[u8; 17]>` in the current scope
2019-11-06T23:21:14.3408989Z    --> src/libcore/fmt/float.rs:40:78
2019-11-06T23:21:14.3410348Z     |
2019-11-06T23:21:14.3410348Z     |
2019-11-06T23:21:14.3411786Z 40  | ...                   sign, precision, false, buf.get_mut(),
2019-11-06T23:21:14.3413341Z     |                                                   ^^^^^^^ method not found in `mem::maybe_uninit::MaybeUninit<[u8; 17]>`
2019-11-06T23:21:14.3416626Z    ::: src/libcore/mem/maybe_uninit.rs:219:1
2019-11-06T23:21:14.3418569Z     |
2019-11-06T23:21:14.3419731Z 219 | pub union MaybeUninit<T> {
2019-11-06T23:21:14.3420922Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3420922Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3422474Z     |
2019-11-06T23:21:14.3423734Z     = help: items from traits can only be used if the trait is implemented and in scope
2019-11-06T23:21:14.3425832Z     = note: the following trait defines an item `get_mut`, perhaps you need to implement it:
2019-11-06T23:21:14.3427601Z             candidate #1: `slice::SliceIndex`
2019-11-06T23:21:14.3428931Z 
2019-11-06T23:21:14.3483438Z error[E0599]: no method named `get_mut` found for type `mem::maybe_uninit::MaybeUninit<[num::flt2dec::Part<'_>; 4]>` in the current scope
2019-11-06T23:21:14.3483998Z    --> src/libcore/fmt/float.rs:41:56
2019-11-06T23:21:14.3485351Z 41  | ...                   parts.get_mut());
2019-11-06T23:21:14.3485351Z 41  | ...                   parts.get_mut());
2019-11-06T23:21:14.3485978Z     |                             ^^^^^^^ method not found in `mem::maybe_uninit::MaybeUninit<[num::flt2dec::Part<'_>; 4]>`
2019-11-06T23:21:14.3486868Z    ::: src/libcore/mem/maybe_uninit.rs:219:1
2019-11-06T23:21:14.3487316Z     |
2019-11-06T23:21:14.3487958Z 219 | pub union MaybeUninit<T> {
2019-11-06T23:21:14.3488558Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3488558Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3488903Z     |
2019-11-06T23:21:14.3489459Z     = help: items from traits can only be used if the trait is implemented and in scope
2019-11-06T23:21:14.3489871Z     = note: the following trait defines an item `get_mut`, perhaps you need to implement it:
2019-11-06T23:21:14.3490273Z             candidate #1: `slice::SliceIndex`
2019-11-06T23:21:14.3558505Z error[E0599]: no method named `get_mut` found for type `mem::maybe_uninit::MaybeUninit<[u8; 1024]>` in the current scope
2019-11-06T23:21:14.3559331Z    --> src/libcore/fmt/float.rs:80:62
2019-11-06T23:21:14.3559685Z     |
2019-11-06T23:21:14.3559685Z     |
2019-11-06T23:21:14.3560089Z 80  | ...                   upper, buf.get_mut(), parts.get_mut());
2019-11-06T23:21:14.3560543Z     |                                  ^^^^^^^ method not found in `mem::maybe_uninit::MaybeUninit<[u8; 1024]>`
2019-11-06T23:21:14.3561261Z    ::: src/libcore/mem/maybe_uninit.rs:219:1
2019-11-06T23:21:14.3561723Z     |
2019-11-06T23:21:14.3562318Z 219 | pub union MaybeUninit<T> {
2019-11-06T23:21:14.3562715Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3562715Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3563024Z     |
2019-11-06T23:21:14.3563412Z     = help: items from traits can only be used if the trait is implemented and in scope
2019-11-06T23:21:14.3563794Z     = note: the following trait defines an item `get_mut`, perhaps you need to implement it:
2019-11-06T23:21:14.3564162Z             candidate #1: `slice::SliceIndex`
2019-11-06T23:21:14.3564923Z 
2019-11-06T23:21:14.3615595Z error[E0599]: no method named `get_mut` found for type `mem::maybe_uninit::MaybeUninit<[num::flt2dec::Part<'_>; 6]>` in the current scope
2019-11-06T23:21:14.3616211Z    --> src/libcore/fmt/float.rs:80:79
2019-11-06T23:21:14.3616639Z     |
2019-11-06T23:21:14.3617127Z 80  | ...                   upper, buf.get_mut(), parts.get_mut());
2019-11-06T23:21:14.3617874Z     |                                                   ^^^^^^^ method not found in `mem::maybe_uninit::MaybeUninit<[num::flt2dec::Part<'_>; 6]>`
2019-11-06T23:21:14.3618523Z    ::: src/libcore/mem/maybe_uninit.rs:219:1
2019-11-06T23:21:14.3618844Z     |
2019-11-06T23:21:14.3619207Z 219 | pub union MaybeUninit<T> {
2019-11-06T23:21:14.3619616Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3619616Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3619925Z     |
2019-11-06T23:21:14.3620302Z     = help: items from traits can only be used if the trait is implemented and in scope
2019-11-06T23:21:14.3620681Z     = note: the following trait defines an item `get_mut`, perhaps you need to implement it:
2019-11-06T23:21:14.3621204Z             candidate #1: `slice::SliceIndex`
2019-11-06T23:21:14.3679709Z error[E0599]: no method named `get_mut` found for type `mem::maybe_uninit::MaybeUninit<[u8; 17]>` in the current scope
2019-11-06T23:21:14.3680360Z    --> src/libcore/fmt/float.rs:100:58
2019-11-06T23:21:14.3682026Z     |
2019-11-06T23:21:14.3682026Z     |
2019-11-06T23:21:14.3682513Z 100 | ...                   buf.get_mut(), parts.get_mut());
2019-11-06T23:21:14.3682996Z     |                           ^^^^^^^ method not found in `mem::maybe_uninit::MaybeUninit<[u8; 17]>`
2019-11-06T23:21:14.3683697Z    ::: src/libcore/mem/maybe_uninit.rs:219:1
2019-11-06T23:21:14.3684497Z     |
2019-11-06T23:21:14.3685019Z 219 | pub union MaybeUninit<T> {
2019-11-06T23:21:14.3685526Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3685526Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3685974Z     |
2019-11-06T23:21:14.3686649Z     = help: items from traits can only be used if the trait is implemented and in scope
2019-11-06T23:21:14.3687259Z     = note: the following trait defines an item `get_mut`, perhaps you need to implement it:
2019-11-06T23:21:14.3688105Z             candidate #1: `slice::SliceIndex`
2019-11-06T23:21:14.3688517Z 
2019-11-06T23:21:14.3740943Z error[E0599]: no method named `get_mut` found for type `mem::maybe_uninit::MaybeUninit<[num::flt2dec::Part<'_>; 6]>` in the current scope
2019-11-06T23:21:14.3741782Z    --> src/libcore/fmt/float.rs:100:75
2019-11-06T23:21:14.3742153Z     |
2019-11-06T23:21:14.3742516Z 100 | ...                   buf.get_mut(), parts.get_mut());
2019-11-06T23:21:14.3743661Z     |                                            ^^^^^^^ method not found in `mem::maybe_uninit::MaybeUninit<[num::flt2dec::Part<'_>; 6]>`
2019-11-06T23:21:14.3744967Z    ::: src/libcore/mem/maybe_uninit.rs:219:1
2019-11-06T23:21:14.3745375Z     |
2019-11-06T23:21:14.3745844Z 219 | pub union MaybeUninit<T> {
2019-11-06T23:21:14.3746372Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3746372Z     | ------------------------ method `get_mut` not found for this
2019-11-06T23:21:14.3746971Z     |
2019-11-06T23:21:14.3747506Z     = help: items from traits can only be used if the trait is implemented and in scope
2019-11-06T23:21:14.3748135Z     = note: the following trait defines an item `get_mut`, perhaps you need to implement it:
2019-11-06T23:21:14.3748539Z             candidate #1: `slice::SliceIndex`
2019-11-06T23:21:14.7016504Z    Compiling std v0.0.0 (/checkout/src/libstd)
2019-11-06T23:21:15.1091213Z    Compiling cmake v0.1.38
2019-11-06T23:21:16.7843204Z error: aborting due to 8 previous errors
2019-11-06T23:21:16.7844440Z 
---
2019-11-06T23:21:17.3132251Z   local time: Wed Nov  6 23:21:17 UTC 2019
2019-11-06T23:21:17.4618091Z   network time: Wed, 06 Nov 2019 23:21:17 GMT
2019-11-06T23:21:17.4618173Z == end clock drift check ==
2019-11-06T23:21:19.0671634Z 
2019-11-06T23:21:19.0788063Z ##[error]Bash exited with code '1'.
2019-11-06T23:21:19.0814802Z ##[section]Starting: Checkout
2019-11-06T23:21:19.0816559Z ==============================================================================
2019-11-06T23:21:19.0816614Z Task         : Get sources
2019-11-06T23:21:19.0816683Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Centril · 2019-11-06T23:22:42Z

Seems there were some errors in other places. Otherwise this looks good. :)

RalfJung · 2019-11-07T09:05:04Z

Likewise; the rename looks good to me but libstd itself actually uses these methods in some places -- incorrectly, FWIW, as things are not yet initialized there. This is old code that was written for mem::uninitialized and would need major refactoring (using raw ptrs and/or MaybeUninit everywhere) to move it to modern standards.

danielhenrymantilla · 2019-11-07T09:58:40Z

there are some errors
I just quickly renamed the basic definitions knowing that it would trigger errors, so as to get them listed here in the PR (I forgot to add WIP to the title).

This is old code that was written for mem::uninitialized and would need major refactoring (using raw ptrs and/or MaybeUninit everywhere) to move it to modern standards.

Then renaming will be a good chance to spot all these usages ;) I don't mind working on more code with you guys overseeing it

Centril · 2019-11-07T10:26:05Z

Do we have FIXMEs in these places? If not, I agree that this is a good opportunity to add some. :)

RalfJung · 2019-11-07T15:53:20Z

I think I added FIXMEs to all of them. If @danielhenrymantilla is up for trying to fix them that's even better of course. ;)

Here's the first one:

rust/src/libcore/fmt/float.rs

Line 15 in 50f8aad

// FIXME(#53491): This is calling `get_mut` on an uninitialized

(and there are a few more in that same file, grep for 53491)

joelpalmer · 2019-11-18T11:49:50Z

Ping from Triage: any updates @danielhenrymantilla?

danielhenrymantilla · 2019-11-18T11:57:08Z

Yes, sorry, I have been quite busy as of late, so this is taking longer than intended: as long as this has [WIP] in its title you can just ignore the PR

SimonSapin · 2019-11-18T14:51:00Z

Renaming sounds good to me. I agree that fixing usage in std at the same time would be ideal, thanks for volunteering @danielhenrymantilla!

JohnCSimon · 2019-12-07T07:42:11Z

Ping from triage:
@danielhenrymantilla - I see your comment about ignoring this PR, but this has been idle for nearly three weeks, so can you please post your status? Thanks.

danielhenrymantilla · 2019-12-13T16:22:13Z

Sure! First of all, and I cannot emphasize this enough,

I apologize for the detail, which has lead to a PR remaining open on this repo for a long time. Sorry.

I have only recently stopped being as busy as I've been for the last 3-4 weeks, so I can go back to work on this in more detail.

More precisely:

renaming all .get_{ref,mut} occurrences by assume_init_{ref,mut} is done very easily, and some may think it would be enough for the PR and the stabilization of these methods to go through. So the option remains of just doing that, and "fixing" the stuff elsewhere. What worries me about that approach, is that we'd effectively be procrastinating the fix.
So, what is to be "fixed"? There is code in the stdlib that "incorrectly" relies on properties of uninitialized memory that Rust has not yet decided to support.
- As an example, ::std::io::Initializer relies on the following code pattern being fine:
```
let mut buf: [MaybeUninit<u8>; N] = MaybeUninit::uninit_array();
Initializer::zeroing().initialize(unsafe { buf.assume_init_mut() });
let mut buf = unsafe { buf.assume_init() };
```
  The second unsafe is sound since .initialize() guarantees that it zeroes the buffer it is fed, buf the first unsafe is not guaranteed to be always sound, since it assumes that a &mut [u8] pointing to initialized memory is valid (also note that when Initializer::nop() is used instead of the zeroing() one, even the second unsafe gets "infected" with this fragile / unstable assumption).
So it is not unsound yet, but it is keeping uninitialized memory semantics from getting more specialized compiler behavior (these semantics, detailed in @RalfJung's blog post: https://www.ralfj.de/blog/2019/07/14/uninit.html, could enable some optimizations), and is leading to choosing instead semantics where the requirement of the u8 being initialized when dealing with &mut u8 be a safety invariant, triggering, for instance, on read-dereferences, rather than a validity invariant.

While such approach is not horrible per se, and some may consider that it is even desirable (to allow a simpler usage of uninitialized memory), the very point of using uninitialized memory is for optimization purposes. So deciding to simplify such code at the cost of missed compiler optimizations kind of defeats the purpose of it. That's mainly my opinion though, and of course such choice of semantics ought to be made elsewhere (c.f. Validity of integers and floating point unsafe-code-guidelines#71).

But even if the choice has not yet been made, I wouldn't like the argument that "the stdlib is currently using the simpler semantics" to be used to defend such semantics. If they are chosen, it must be done to simplify future code, not to validate past code that, at the time it was written, relied on a language feature that was never guaranteed.

That's why changing this to only relying on &mut [MaybeUninit<u8>] write-slices, for instance, is definitely something that would greatly benefit not only the stdlib but also the future development of the language. And that's what I am doing. As a comparison point, I'm the author of the ::uninit crate, whose purpose is precisely this one. So I already have a draft of the design required to make all this work (which has led me to stumbling upon a mistake that made one method of the crate unsound, so at least that is known from now on, see below for a detail about it).

`Read` / `Initializer` rework

So, one of the main issues here is the Read trait and its .read() method

(there are also some issues when formatting floats, but I'll work on them after the Read trait)

trait Read {
    fn read (self: &'_ mut Self, buf: &'_ mut [u8]) -> Result<usize>;
    ...
}

and to handle uninitialized buffers, the trait adds an extra unsafe fn initializer() -> Initializer associated function, which returns one of two instances of Initializer, the one that zeroes uninitialized buffers, or the one that lets them as is. The idea being that with the previous pattern shown, if the read() implementation was known not to read from buf (assuming safety invariant instad of a validity one), then using Initializer::nop() is fine.
- Additionally, most of these functions were made unsafe to make sure somebody using the .nop() optimization despite them reading from buf had to use an unsafe somewhere. But as I said, this locks us into that choice of semantics.

So, in hindsight, this trait's signature is too broad: it should, instead, have been

trait Read {
    fn read (self:&'_ mut Self, buf: &'_ out [u8]) -> Result<usize>;
    ...
}

where &'out [u8] would be a write-only (and unaliased) Rust reference to a slice of bytes (i.e., it must be imposible, without unsafe, to read from buf). Given its write-only semantics, it would be fine have such reference point to a [u8] as well as a [MaybeUninit<u8>]. So all is fine.

But we don't have &out references!

Well, we don't have them at the language level, and maybe all this could be seen as a strong motivator for pushing their development, but anyways, we can implement &out references as a library type.

Technical requirement: `Out<'_, T>` and `OutSlice<'_, T>` abstractions

Now, here we may too promptly say that type Out<'a, T> = &'a mut MaybeUninit<T> (and ditto for slice references) may be enough, since &mut MaybeUninit<_> is a write-only unaliased reference to memory that may or may not be initialized. I, for instance, made that mistake, and is the reason my current implementation in ::uninit can lead to unsoundness for users of the library.

To see why,

I'll first take @RalfJung theoretical description of the mistake: while it is true that a T can be seen as some abstract subtype of a MaybeUninit<T> (MaybeUninit::new being the "upcast" operator), such "subtyping" ceases to be valid when behind a &mut reference, given the invariance of &mut.

Now, for a more concrete example,

fn unsound<T> (p: &mut T) -> &mut MaybeUninit<T> { unsafe { transmute(p) } }

fn exploit<T> (mut x: T) -> T
{
    *unsound(&mut x) = MaybeUninit::uninitialized();
    x // returns uninitialized::<T>() !
}

So being write-only is too strong of an PI, the API needs to be "write-T-only". So we newtype &'a mut MaybeUninit<T> and only allow non-unsafe APIs that write initialized Ts there.

Also, as a cautionary restriction, we can add a Copy bound on T to avoid leaking memory too easily. In our case T = u8 so it suffices.

`ReadOut` / `ReadIntoUninit` / `BikeshedName` extension trait to the rescue:

Following the ::uninit crate, I'll name it ReadIntoUninit in the following explanation:

unsafe trait ReadIntoUninit : Read {
    fn read_into_uninit<'buf> (self: &'_ mut Self, buf: impl Into<OutSlice<'buf, u8>> + 'buf)
      -> Result<&'buf mut [u8]>
    ;
    ...
}

unsafe impl<R : ?Sized + Read> ReadIntoUninit for R {
    default
    fn read_into_uninit<'buf> (self: &'_ mut Self, buf: impl Into<OutSlice<'buf, u8>> + 'buf)
      -> Result<&'buf mut [u8]>
    {
        let mut buf = buf.into();
        let buf: &mut [u8] = buf.write_all(&0); // Initializer::zeroing().initialize(&mut buf) here
        self.read(buf).map(|n| &mut buf[.. n])
    }
}

For those wondering why the trait is (sadly) required to be unsafe, I'll come to it in a minute.

This way:

the implementations that used to return Initializer::nop can instead:

unsafe impl ReadIntoUninit for MyType {
    fn read_into_uninit<'buf> (self:&'_ mut Self, buf: impl Into<OutSlice<'buf, u8>> + 'buf)
      -> Result<&'buf mut [u8]>
    {
        let mut buf = buf.into();
        // implementaiton goes here
    }
}

impl Read for MyType {
    fn read (self: &'_ mut Self, buf: &'_ mut [u8])
      -> Result<usize>
    {
        self.read_into_uninit(buf).map(|slice| slice.len())
    }
}

and the initialization pattern will now be:

let mut buf: [MaybeUninit<u8>; N] = MaybeUninit::uninit_array();
let _ = reader.read_into_uninit_exact(&mut buf[..])?;
unsafe {
    // Safety: `ReadIntoUninit`'s safety guarantees ensure that `buf` has been initialized.
    buf.assume_init()
}

And now you can see why the trait needs to be unsafe: the implementations need to guarantee that the buffers they have been given have been initialized. For more details about these safety guarantees, you can refer to my crate's documentation: https://docs.rs/uninit/0.1.0/uninit/trait.ReadIntoUninit.html#safety

TL,DR / What does it mean in practice

Well this post has been longer than I initially expected. I hope it shows that correctly fixing the .get_mut() issues is non-triviable but definitely doable.

So, what should we do? Given that the design changes seem too important for such an innocent-looking PR as this one, I suggest the following (but of course, you decide):

(temporarily) close this PR;
I submit a new PR, focusing only on these design changes, I guess with an RFC and everything;
Imagining that PR and everything involved is accepted (meaning that the "wrong usages" of .get_mut() are "fixed"), we can reopen this PR, rebase after that one, and do the renaming.

Alternative:

We "abandon" the fixing attempt, and just rename the methods, getting the PR merged.
I submit a new PR, focusing on "fixing" the now accepted PR (I guess with an RFC and everything).

The alternative looks more appealing at first glance, but I am worried that if this PR gets merged, then the motivation behind "fixing" the .get_mut()s vanishes / gets procrastinated again, and something like next year we just end up going for the "simpler uninit semantics" choice and road.

RalfJung · 2019-12-21T13:43:14Z

I agree with separating out the Read changes. That's API design stuff and is currently being worked on by other as AsyncRead is being discussed, so I suggest you join that discussion and we keep this PR out of it.

But no need to close the PR, the rename still makes sense! If you can't get around to fixing all the existing users, that's okay as well, nothing actually gets worse from this PR. If you could fix all users except for Read, that would be perfect.

spunit262 · 2019-12-22T05:21:32Z

Technical requirement: Out<'_, T> and OutSlice<'_, T> abstractions

Going &mut T to &mut MaybeUninit<T> is always unsound, but &mut MaybeUninit<T> to &mut T is sound if you know for a fact it's a T (because you put a T there yourself). This is in fact exactly what MaybeUninit::write does, and Option::get_or_insert does something logicaly similar too.

Dylan-DPC-zz · 2020-01-15T08:54:48Z

I think it is better to split this in separate prs. Closing this. Thanks for taking the time to contribute :)

aentity · 2020-08-24T19:02:35Z

Hi, its not clear to me what happened; the issue was closed, but no follow up PR was proposed or done. This feature is now in limbo?

RalfJung · 2020-08-25T07:49:44Z

This is not an issue, it is a PR. It got closed, IMO because of feature creep, and nobody else showed up to finish the work.

The feature is still tracked at #63568.

@RalfJung

rename get_{ref, mut} to assume_init_{ref,mut} in Maybeuninit References rust-lang#63568 Rework with comments addressed from rust-lang#66174 Have replaced most of the occurrences I've found, hopefully didn't miss out anything r? @RalfJung (thanks @danielhenrymantilla for the initial work on this)

rust-highfive assigned RalfJung Nov 6, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 6, 2019

danielhenrymantilla changed the title ~~Rename MaybeUninit::get_{ref,mut} to MaybeUninit::assume_init_{ref,mut}~~ [WIP] Rename MaybeUninit::get_{ref,mut} to MaybeUninit::assume_init_{ref,mut} Nov 10, 2019

RalfJung added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2019

dtolnay mentioned this pull request Nov 27, 2019

Tracking issue for #![feature(maybe_uninit_ref)] #63568

Closed

Dylan-DPC-zz closed this Jan 15, 2020

Dylan-DPC-zz mentioned this pull request Aug 29, 2020

rename get_{ref, mut} to assume_init_{ref,mut} in Maybeuninit #76047

Merged

danielhenrymantilla mentioned this pull request Aug 30, 2020

Fix incorrect use of MaybeUninit::assume_init_mut in flt2dec #76092

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Rename MaybeUninit::get_{ref,mut} to MaybeUninit::assume_init_{ref,mut} #66174

[WIP] Rename MaybeUninit::get_{ref,mut} to MaybeUninit::assume_init_{ref,mut} #66174

danielhenrymantilla commented Nov 6, 2019

rust-highfive commented Nov 6, 2019

Centril commented Nov 6, 2019

RalfJung commented Nov 7, 2019

danielhenrymantilla commented Nov 7, 2019

Centril commented Nov 7, 2019

RalfJung commented Nov 7, 2019

joelpalmer commented Nov 18, 2019

danielhenrymantilla commented Nov 18, 2019

SimonSapin commented Nov 18, 2019

JohnCSimon commented Dec 7, 2019

danielhenrymantilla commented Dec 13, 2019 •

edited

Loading

RalfJung commented Dec 21, 2019

spunit262 commented Dec 22, 2019

Dylan-DPC-zz commented Jan 15, 2020

aentity commented Aug 24, 2020

RalfJung commented Aug 25, 2020

[WIP] Rename MaybeUninit::get_{ref,mut} to MaybeUninit::assume_init_{ref,mut} #66174

[WIP] Rename MaybeUninit::get_{ref,mut} to MaybeUninit::assume_init_{ref,mut} #66174

Conversation

danielhenrymantilla commented Nov 6, 2019

rust-highfive commented Nov 6, 2019

Centril commented Nov 6, 2019

RalfJung commented Nov 7, 2019

danielhenrymantilla commented Nov 7, 2019

Centril commented Nov 7, 2019

RalfJung commented Nov 7, 2019

joelpalmer commented Nov 18, 2019

danielhenrymantilla commented Nov 18, 2019

SimonSapin commented Nov 18, 2019

JohnCSimon commented Dec 7, 2019

danielhenrymantilla commented Dec 13, 2019 • edited Loading

Read / Initializer rework

Technical requirement: Out<'_, T> and OutSlice<'_, T> abstractions

ReadOut / ReadIntoUninit / BikeshedName extension trait to the rescue:

TL,DR / What does it mean in practice

RalfJung commented Dec 21, 2019

spunit262 commented Dec 22, 2019

Dylan-DPC-zz commented Jan 15, 2020

aentity commented Aug 24, 2020

RalfJung commented Aug 25, 2020

danielhenrymantilla commented Dec 13, 2019 •

edited

Loading

`Read` / `Initializer` rework

Technical requirement: `Out<'_, T>` and `OutSlice<'_, T>` abstractions

`ReadOut` / `ReadIntoUninit` / `BikeshedName` extension trait to the rescue: